Pass-through options from DOM.context2d to getContext #273

Fil · 2021-12-20T14:58:33Z

The third argument to DOM.context2d can be an options object with {scale, colorSpace}, allowing the creation of wide-gamut graphics on supported hardware and software.

Note: this necessitates a try/catch because Safari will throw if the os requirements are not met.

Tests/demo: https://observablehq.com/@fil/colorspace-display-p3-context2d

Draft, mostly for discussion.

mbostock

We shouldn’t code for specific options and should instead support passthrough. If options is an object, we pass it through to canvas.getContext, and we don’t add any try-catch around it. For backwards compatibility, if options is a number, it’s promoted to an options object {scale}.

Although, I’m also tempted to do nothing here. For the most part the DOM API in standard library is deprecated. It’s better to just have people write against the standard APIs so that when code is copy-pasted outside of Observable there are no gratuitous dependencies on the standard library.

Fil · 2021-12-20T17:40:46Z

It's fine to just do nothing. Easy enough to make this helper outside of stlib for special cases. (But it's very useful in general to not have to worry about dpr.)

mbostock · 2021-12-20T17:55:06Z

src/dom/context2d.js

+  if (options == null) {
+    options = {};


It’d be better to be strict here and only promote options to an object when it is undefined, ideally using default arguments (options = {}). If the user passes in null explicitly, we should respect that and pass null to canvas.getContext.

mbostock · 2021-12-20T17:55:29Z

src/dom/context2d.js

-  if (options == +options) {
+  if (options == null) {
+    options = {};
+  } else if (options == +options) {


This should be strict based on the type (typeof options === "number") rather than using coercion.

Trying to be fully compatible with the current implementation, I wanted to support dpr passed as a string. Not sure why anyone would do that, though :)

Full compatibility is not practical; someone could pass an object that implements valueOf, and this would have worked with the old API, but is ambiguous with named options under the new API. We should only support the intended pattern of passing a number.

mbostock · 2021-12-20T17:56:10Z

src/dom/context2d.js

    scale = +options;
+    options = {};


This is probably fine, but it’d be slightly better to pass undefined, I think.

src/dom/context2d.js

Fil · 2022-10-05T14:58:19Z

we might want to document the willReadFrequently boolean option as well?

…ale, colorSpace}, allowing the creation of wide-gamut graphics on supported hardware and software. Note: this necessitates a try/catch because Safari will throw if the os requirements are not met. Tests/demo: https://observablehq.com/@fil/colorspace-display-p3-context2d

mootari · 2022-10-05T15:22:15Z

I believe the options handling can be simplified:

export default function(width, height, options = {}) {
  const {
    scale = devicePixelRatio,
    ...contextOptions
  } = !isNaN(options) ? {scale: options} : options;
  const canvas = document.createElement("canvas");
  canvas.width = width * scale;
  canvas.height = height * scale;
  canvas.style.width = width + "px";
  const context = canvas.getContext("2d", contextOptions);
  context.scale(scale, scale);
  return context;
}

mootari · 2022-10-05T16:08:14Z

src/dom/context2d.mjs

+  const {scale = devicePixelRatio, ...contextOptions} = !isNaN(options)
+    ? {...(options != null && {scale: options})}
+    : options;


Suggested change

const {scale = devicePixelRatio, ...contextOptions} = !isNaN(options)

? {...(options != null && {scale: options})}

: options;

const {scale = devicePixelRatio, ...contextOptions} = options && !isNaN(options)

? {scale: +options}

: options;

0 is currently working as before (in the sense that it returns a canvas of size 0)—it would be simpler if we didn't support this "feature".

With the additional null check in the earlier comment we can remove options && here.

(btw, that parseFloat was the result of a brainfart - I meant to write +options)

src/dom/context2d.mjs

mootari · 2022-10-05T16:31:38Z

README.md

@@ -41,9 +41,9 @@ html`<canvas width=960 height=500>`

 If you are using [2D Canvas](https://www.w3.org/TR/2dcontext/) (rather than [WebGL](https://webglfundamentals.org/)), you should use [DOM.context2d](#DOM_context2d) instead of DOM.canvas for automatic pixel density scaling.

-<a href="#DOM_context2d" name="DOM_context2d">#</a> DOM.<b>context2d</b>(<i>width</i>, <i>height</i>[, <i>dpi</i>]) [<>](https://github.com/observablehq/stdlib/blob/main/src/dom/context2d.mjs "Source")
+<a href="#DOM_context2d" name="DOM_context2d">#</a> DOM.<b>context2d</b>(<i>width</i>, <i>height</i>[, <i>options</i>]) [<>](https://github.com/observablehq/stdlib/blob/main/src/dom/context2d.mjs "Source")


Shouldn't we list both signatures here?

It's there: "As a shorthand notation, options having only scale can be specified as a number." (could be phrased better?)

I feel the shorthand should be mentioned earlier and emphasized, since it's the format that you'll encounter throughout Observable and the notation that you'll commonly use to disable dpr scaling.

mootari · 2022-10-05T16:34:12Z

src/dom/context2d.mjs

-  canvas.height = height * dpi;
+export default function (width, height, options = {}) {
+  const {scale = devicePixelRatio, ...contextOptions} = !isNaN(options)
+    ? {...(options != null && {scale: options})}


There's no need for the destructuring here, and the null check should be explicit. If options is undefined, we already get {}.

Suggested change

? {...(options != null && {scale: options})}

? options === null ? {} : {scale: options}

Fil · 2023-03-23T08:21:36Z

Chrome (>= 111) supports display-p3
https://developer.chrome.com/articles/high-definition-css-color-guide/

Fil requested a review from mbostock December 20, 2021 14:58

mbostock requested changes Dec 20, 2021

View reviewed changes

Fil changed the title ~~colorSpace: "display-p3"~~ Pass-through options from DOM.context2d to getContext Dec 20, 2021

mbostock requested changes Dec 20, 2021

View reviewed changes

Fil added 3 commits October 5, 2022 17:17

pass-through

c82eaa8

cleaner

4e72641

Fil force-pushed the fil/colorSpace branch from 017a548 to 4e72641 Compare October 5, 2022 15:19

Apply Fabian’s suggestion, handle null options

76e1b11

mootari self-requested a review October 5, 2022 16:06

Fil requested a review from mbostock October 5, 2022 16:08

Fil marked this pull request as ready for review October 5, 2022 16:09

mootari requested changes Oct 5, 2022

View reviewed changes

options === null

e1b6ea9

mootari reviewed Oct 5, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass-through options from DOM.context2d to getContext #273

Pass-through options from DOM.context2d to getContext #273

Fil commented Dec 20, 2021

mbostock left a comment

Fil commented Dec 20, 2021

mbostock Dec 20, 2021

mbostock Dec 20, 2021

Fil Dec 20, 2021

mbostock Dec 20, 2021

mbostock Dec 20, 2021

Fil commented Oct 5, 2022 •

edited

Loading

mootari commented Oct 5, 2022 •

edited

Loading

mootari Oct 5, 2022 •

edited

Loading

Fil Oct 5, 2022

mootari Oct 5, 2022

mootari Oct 5, 2022

mootari Oct 5, 2022

Fil Oct 5, 2022

mootari Oct 5, 2022

mootari Oct 5, 2022 •

edited

Loading

Fil commented Mar 23, 2023

	? {...(options != null && {scale: options})}
	? options === null ? {} : {scale: options}

Pass-through options from DOM.context2d to getContext #273

Are you sure you want to change the base?

Pass-through options from DOM.context2d to getContext #273

Conversation

Fil commented Dec 20, 2021

mbostock left a comment

Choose a reason for hiding this comment

Fil commented Dec 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fil commented Oct 5, 2022 • edited Loading

mootari commented Oct 5, 2022 • edited Loading

mootari Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mootari Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Fil commented Mar 23, 2023

Fil commented Oct 5, 2022 •

edited

Loading

mootari commented Oct 5, 2022 •

edited

Loading

mootari Oct 5, 2022 •

edited

Loading

mootari Oct 5, 2022 •

edited

Loading